Automated Review -- Tests Failed

1. Root Cause of the Test Failure

The test failure is caused by a database connection error during Odoo startup, which is unrelated to the code changes in this PR. The log shows that the database connection failed, likely due to an infrastructure or configuration issue in the test environment (e.g., runboat-infra.svc.cluster.local not reachable). This is not a bug in the module logic.

2. Suggested Fix

There is no fix needed for the code in this PR, as the failure is due to an external environment issue, not the module's logic.

However, to ensure future stability in CI environments, consider:

• Adding a retry mechanism or health check in CI scripts before running tests.
• Ensuring that the test database (runboat@postgres.runboat-infra.svc.cluster.local) is accessible and stable.

3. Additional Code Issues

a. Incorrect use of sudo() on self in _action_done()

• File: stock_picking.py
• Line: ~150
• Issue: for pick in self.filtered(...).sudo():
• Problem: Calling .sudo() on self (a recordset) is not recommended. The sudo() should be applied to the operations inside the loop, not the recordset itself.
• Suggested Fix:

  for pick in self.filtered(...):
      pick = pick.sudo()
      ...

b. Inconsistent use of self.sudo() in _get_intercompany_move_lots

• File: stock_picking.py
• Line: ~35
• Issue: sale_lot.get_inter_company_lot(...) is called with self.sudo() on the lot, but get_inter_company_lot is a method on stock.lot, not a model-level method that requires sudo.
• Suggested Fix: Remove self.sudo() from the call:

  po_move_lots |= sale_lot.get_inter_company_lot(...)

c. Potential performance issue in _sync_return_quantities

• File: stock_picking.py
• Line: ~180
• Issue: Nested loop with for po_ml in po_move_lines: and if qty_to_set <= 0: break may not scale well for large move lines.
• Suggested Fix: Optimize the loop to avoid redundant iterations or pre-filter po_move_lines.

4. Test Improvements

a. Add test coverage for return picking logic

• Issue: The new return logic in _action_done() is not fully tested.
• Suggestion: Add a test case that creates a return from customer, ensures the intercompany flow works correctly, and validates the lot assignment and quantities.

b. Test edge cases for lot creation modes

• Issue: Lot creation modes (same, manual) are tested, but edge cases like missing lots or mismatched quantities are not covered.
• Suggestion: Add test cases for:
  • No lot assigned in manual mode → should raise UserError
  • Lot not found in get_inter_company_lot → should create new lot
  • Partial quantities in return picking

c. Use SavepointCase for data isolation

• Suggestion: Use odoo.tests.SavepointCase instead of TransactionCase where appropriate to ensure clean data per test method, especially for complex inter-company flows.

d. Tag tests with meaningful tags

• Suggestion: Tag tests with @tag('intercompany', 'lot', 'return') to improve test organization and selection.

Summary

• The test failure is environment-related, not code-related.
• Code changes are mostly correct, but some sudo() usage and performance considerations should be reviewed.
• Tests are good but can be improved with edge case coverage and better tagging.

⏰ This PR has been open for 32 days.
🔍 No human reviews yet after 32 days. PSC members: a quick review would help keep this contributor engaged with OCA.
💤 Last activity was 32 days ago.

Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)

Reciprocal Review Request

Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):

My open PRs across OCA:

• server-tools#3554 [MIG] datetime_formatter: Migration to 18.0
• server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
• hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
• stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
• stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
• stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
• purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix

Reviewing each other's work helps the whole community move forward. Thank you!

Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b